Skip to content

refactor: remove static_files.to_settings() and add edge feature to RocksDB flags#21225

Merged
Rjected merged 1 commit intomainfrom
fix/remove-static-files-to-settings
Jan 21, 2026
Merged

refactor: remove static_files.to_settings() and add edge feature to RocksDB flags#21225
Rjected merged 1 commit intomainfrom
fix/remove-static-files-to-settings

Conversation

@gakonst
Copy link
Copy Markdown
Member

@gakonst gakonst commented Jan 20, 2026

Summary

This PR addresses the review comment from #21191 (discussion):

we can do it in this PR or on a follow-up, but we probably want to remove static_files.to_settings() i think ?
and we should make sure that the following behaviour introduced here #21208 also applies to rocksdb

Changes

  1. Remove StaticFilesArgs::to_settings() - This method is now superseded by NodeConfig::storage_settings() which handles both static files and RocksDB settings in one place.

  2. Add edge feature behavior to RocksDB flags - Following the pattern from fix: set StaticFileArgs defaults for edge #21208:

    • Individual RocksDB flags (tx_hash, storages_history, account_history) now default to true when the edge feature is enabled
    • Changed flags from Option<bool> to bool with proper defaults using default_value_t and ArgAction::Set
    • Added consistent doc comments about genesis-initialization-only behavior
  3. Add RocksDbArgs to EnvironmentArgs - The EnvironmentArgs in cli/commands now includes RocksDbArgs with a storage_settings() method for consistent storage settings across all CLI commands.

  4. Update storage_settings() to use edge feature base - Both NodeConfig::storage_settings() and EnvironmentArgs::storage_settings() now start from StorageSettings::edge() or StorageSettings::legacy() based on the feature flag.

Files Changed

  • crates/node/core/src/args/rocksdb.rs - Add edge feature defaults, change Option to bool
  • crates/node/core/src/args/static_files.rs - Remove to_settings() method
  • crates/node/core/src/node_config.rs - Update storage_settings() to use edge feature base
  • crates/cli/commands/src/common.rs - Add RocksDbArgs and storage_settings()
  • crates/node/builder/src/launch/common.rs - Update callers to use storage_settings()

@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Jan 20, 2026
Base automatically changed from feat/rocksdb-cli-v2 to main January 20, 2026 19:40
Copy link
Copy Markdown
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs rebase / conflicts fixed

Comment on lines +78 to +81
#[cfg(feature = "edge")]
let base = StorageSettings::edge();
#[cfg(not(feature = "edge"))]
let base = StorageSettings::legacy();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to wrap this in a fn

@gakonst gakonst force-pushed the fix/remove-static-files-to-settings branch 4 times, most recently from 82b3964 to 815895e Compare January 21, 2026 14:40
…ocksDB flags

This PR addresses the review comment from #21191:

- Remove `StaticFilesArgs::to_settings()` - superseded by `NodeConfig::storage_settings()`
- Add edge feature behavior to RocksDB flags - flags default to `true` when `edge` feature is enabled
- Add `RocksDbArgs` to `EnvironmentArgs` with `storage_settings()` method
- Update `storage_settings()` to use `StorageSettings::edge()` or `StorageSettings::legacy()` base
- Replace `static_files.to_settings()` calls with `storage_settings()` in launch/common.rs
@gakonst gakonst force-pushed the fix/remove-static-files-to-settings branch from 815895e to a7aaafa Compare January 21, 2026 15:26
@gakonst gakonst requested a review from joshieDo as a code owner January 21, 2026 15:26
Copy link
Copy Markdown
Collaborator

@joshieDo joshieDo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Rjected Rjected added this pull request to the merge queue Jan 21, 2026
Merged via the queue into main with commit dd72cfe Jan 21, 2026
45 checks passed
@Rjected Rjected deleted the fix/remove-static-files-to-settings branch January 21, 2026 17:01
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Jan 21, 2026
@yongkangc yongkangc self-assigned this Jan 22, 2026
@yongkangc yongkangc added the A-rocksdb Related to rocksdb integration label Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-rocksdb Related to rocksdb integration

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants